Skip to content

feat(PageLayout.Sidebar): support controlled width via currentWidth + onResizeEnd#7900

Open
mattcosta7 wants to merge 3 commits into
mainfrom
page-layout-sidebar-controlled-width
Open

feat(PageLayout.Sidebar): support controlled width via currentWidth + onResizeEnd#7900
mattcosta7 wants to merge 3 commits into
mainfrom
page-layout-sidebar-controlled-width

Conversation

@mattcosta7
Copy link
Copy Markdown
Contributor

Closes #

Adds controlled-width support to PageLayout.Sidebar (and transitively SplitPageLayout.Sidebar), matching the discriminated-union API that already exists on PageLayout.Pane. The underlying usePaneWidth hook already accepted these options — this change only exposes them on the Sidebar prop surface and forwards them through.

Changelog

New

  • PageLayout.Sidebar and SplitPageLayout.Sidebar now accept a controlled-width pair:
    • currentWidth: number | undefined — the displayed pane width (in pixels); the existing width prop continues to define the default used on reset/double-click.
    • onResizeEnd: (width: number) => void — fires at the end of a drag or keyboard resize gesture; when supplied, localStorage persistence is bypassed entirely (consumer owns persistence).
    • Types use the same discriminated-union pattern as PageLayoutPaneProps — either both are required together or both must be omitted.
  • Storybook stories:
    • PageLayout/Features/ResizableSidebarWithoutPersistence — in-memory controlled state, no persistence.
    • PageLayout/Features/ResizableSidebarWithCustomPersistence — controlled state backed by a consumer-managed localStorage key.
  • Unit tests in PageLayout.test.tsx:
    • Asserts the controlled currentWidth is written into the --pane-width CSS variable on the sidebar element.
    • Asserts onResizeEnd is invoked exactly once after a keyboard resize gesture, with a numeric width.
  • Doc entries (PageLayout.docs.json) for currentWidth, onResizeEnd, and the previously-undocumented widthStorageKey prop on Sidebar.

Changed

  • PageLayoutSidebarProps is now PageLayoutSidebarBaseProps & ({onResizeEnd, currentWidth} | {onResizeEnd?: never, currentWidth?: never}) — mirroring PageLayoutPaneProps. The non-controlled shape is unchanged; existing call sites continue to compile.
  • Sidebar's suppressHydrationWarning becomes resizable === true && !!widthStorageKey && !onResizeEnd — only set when a hydration mismatch is actually possible (server can't read localStorage). Sidebar's opt-in storage model means !!widthStorageKey stays in the predicate (unlike Pane, which defaults widthStorageKey to 'paneWidth').
  • Sidebar's widthStorageKey JSDoc now mentions that it is bypassed when onResizeEnd is provided, matching Pane.

Removed

Nothing public.

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

(The changeset is published as a minor bump because it adds two new public props on a public component; flag if the team prefers patch.)

Testing & Reviewing

  • All 86 unit tests in packages/react/src/PageLayout/** and packages/react/src/SplitPageLayout/** pass (2 pre-existing todos).
  • `tsc --noEmit` on `packages/react` is clean.
  • Prettier + ESLint clean on touched files.
  • Existing uncontrolled stories (`ResizableSidebar`, `SidebarWithPaneResizable`, etc.) are byte-identical at default rendering — only the two new stories exercise the controlled-width branch.

The new stories are the easiest manual check: drag or arrow-key the divider on ResizableSidebarWithoutPersistence and confirm the displayed width in the placeholder label updates on release; reload ResizableSidebarWithCustomPersistence and confirm the previously-set width is restored from localStorage.

Merge checklist

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 29, 2026

🦋 Changeset detected

Latest commit: 87d9f7b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@mattcosta7 mattcosta7 marked this pull request as ready for review May 29, 2026 12:46
@mattcosta7 mattcosta7 requested a review from a team as a code owner May 29, 2026 12:46
@mattcosta7 mattcosta7 requested review from Copilot and siddharthkp May 29, 2026 12:46
@mattcosta7 mattcosta7 self-assigned this May 29, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds controlled-width support to PageLayout.Sidebar, aligning it with the existing PageLayout.Pane controlled resize API and flowing through to SplitPageLayout.Sidebar.

Changes:

  • Introduces currentWidth + onResizeEnd discriminated-union props for PageLayout.Sidebar.
  • Forwards controlled resize options into usePaneWidth and adjusts hydration suppression for persistence-only cases.
  • Adds tests, Storybook examples, docs metadata, and a changeset for the new public API.
Show a summary per file
File Description
packages/react/src/PageLayout/PageLayout.tsx Adds Sidebar controlled-width props and forwards them to resize state handling.
packages/react/src/PageLayout/PageLayout.test.tsx Adds component-level coverage for controlled width rendering and resize-end callback behavior.
packages/react/src/PageLayout/PageLayout.features.stories.tsx Adds controlled Sidebar Storybook examples with in-memory and custom persistence.
packages/react/src/PageLayout/PageLayout.docs.json Documents new Sidebar props and related resizable behavior.
.changeset/pagelayout-sidebar-controlled-width.md Adds a minor changeset for the public API addition.

Copilot's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment thread .changeset/pagelayout-sidebar-controlled-width.md
Comment thread packages/react/src/PageLayout/PageLayout.features.stories.tsx Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@mattcosta7 mattcosta7 enabled auto-merge May 29, 2026 17:51
@primer primer Bot disabled auto-merge May 29, 2026 20:11
@primer primer Bot enabled auto-merge May 29, 2026 20:11
@primer primer Bot disabled auto-merge May 29, 2026 20:13
@primer primer Bot enabled auto-merge May 29, 2026 20:13
@primer-integration
Copy link
Copy Markdown

⚠️ Hi from github/github-ui! The integration workflow could not find a canary version for the latest commit on this PR.

A successful canary CI run (i.e., a valid canary version published via the release.yml workflow) must exist for the latest commit before integration checks will succeed.

Next steps:

  1. Make sure the Canary Release label is applied to the PR — the release.yml workflow requires this label to publish a canary version.
  2. Wait for the release.yml canary CI run to complete successfully for the latest commit on this PR.
  3. Once a valid canary version exists, re-trigger the integration workflow by visiting the primer-react-pr-test workflow page, clicking Run workflow, and pasting this PR's URL.

For more details, see this workflow run.

@mattcosta7 mattcosta7 added the Canary Release Apply this label when you want CI to create a canary release of the current PR label Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants